fix(server): remove initialized notification gate to support Streamable HTTP#788
Open
anara123 wants to merge 2 commits intomodelcontextprotocol:mainfrom
Open
fix(server): remove initialized notification gate to support Streamable HTTP#788anara123 wants to merge 2 commits intomodelcontextprotocol:mainfrom
anara123 wants to merge 2 commits intomodelcontextprotocol:mainfrom
Conversation
…le HTTP The server's init handshake loop fatally rejected any request arriving before the `notifications/initialized` message. This breaks Streamable HTTP clients where each JSON-RPC message is a separate POST with no ordering guarantee — `tools/list` can easily arrive before `initialized`. Remove the ~40-line wait loop and enter `serve_inner` immediately after sending `InitializeResult`. The `initialized` notification is now handled as a regular notification by the main service loop, matching the TypeScript SDK behavior (validated in typescript-sdk#578). Also remove the now-unreachable `ExpectedInitializedNotification` error variant from `ServerInitializeError`. Closes modelcontextprotocol#783 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
github-actions
bot
added
T-test
Merged
3 tasks
Contributor
Author
|
@DaleSeo @alexhancock can anyone review this please. |
DaleSeo
reviewed
Apr 9, 2026
| ExpectedInitializeRequest(Option<ClientJsonRpcMessage>), | ||
|
|
||
| #[error("expect initialized notification, but received: {0:?}")] | ||
| ExpectedInitializedNotification(Option<ClientJsonRpcMessage>), |
Member
There was a problem hiding this comment.
Since ServerInitializeError is public, removing its variant is a breaking change. Since the behavior changed intentionally, I suggest keeping this variant as deprecated. This way, we can maintain compatibility and avoid bumping the major version in the next release.
Here's the output from the semver checks:
$ cargo semver-checks check-release
Building rmcp v1.3.0 (current)
Built [ 17.264s] (current)
Parsing rmcp v1.3.0 (current)
Parsed [ 0.069s] (current)
Building rmcp v1.3.0 (baseline)
Built [ 3.207s] (baseline)
Parsing rmcp v1.3.0 (baseline)
Parsed [ 0.066s] (baseline)
Checking rmcp v1.3.0 -> v1.3.0 (no change; assume patch)
Checked [ 0.082s] 221 checks: 220 pass, 1 fail, 0 warn, 24 skip
--- failure enum_variant_missing: pub enum variant removed or renamed ---
Description:
A publicly-visible enum has at least one variant that is no longer available under its prior name. It may have been renamed or removed entirely.
ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.46.0/src/lints/enum_variant_missing.ron
Failed in:
variant ServerInitializeError::ExpectedInitializedNotification, previously in file /private/var/folders/v2/5j2zr9qs7c76ph0yxn53d_9r0000gp/T/cursor-sandbox-cache/ef50fef5e15a857ee43162e39176828f/cargo-target/semver-checks/git-8e22aa2de28df5a285eed87c11cd89bf15fa90d3/257a621cff1beb7eafa5cd4a260f415999422844/crates/rmcp/src/service/server.rs:57
Summary semver requires new major version: 1 major and 0 minor checks failed
Finished [ 22.930s] rmcp
Contributor
Author
There was a problem hiding this comment.
Thanks for the review, good catch. I reverted this variant.
Member
There was a problem hiding this comment.
Thanks for addressing this, @anara123! Now I see it will bump the minor version. 👍
$ cargo semver-checks check-release
Building rmcp v1.3.0 (current)
Built [ 3.137s] (current)
Parsing rmcp v1.3.0 (current)
Parsed [ 0.072s] (current)
Building rmcp v1.3.0 (baseline)
Built [ 2.607s] (baseline)
Parsing rmcp v1.3.0 (baseline)
Parsed [ 0.067s] (baseline)
Checking rmcp v1.3.0 -> v1.3.0 (no change; assume patch)
Checked [ 0.074s] 221 checks: 220 pass, 1 fail, 0 warn, 24 skip
--- failure enum_variant_marked_deprecated: enum variant #[deprecated] added ---
Description:
An enum variant is now #[deprecated]. Downstream crates will get a compiler warning when using this variant.
ref: https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-deprecated-attribute
impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.46.0/src/lints/enum_variant_marked_deprecated.ron
Failed in:
variant rmcp::service::ServerInitializeError::ExpectedInitializedNotification in /Users/dale.seo/.cursor/worktrees/rust-sdk/review-pr-788-qd1/crates/rmcp/src/service/server.rs:61
Summary semver requires new minor version: 0 major and 1 minor checks failed
Finished [ 9.655s] rmcp
Retain the variant for semver compatibility — removing it would be a breaking change caught by cargo-semver-checks. Mark it deprecated with a note that it is never constructed and will be removed in a future major release. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
DaleSeo
approved these changes
Apr 9, 2026
Member
|
@anara123 We have an issue with the release checks in main. I'll fix that first before merging this. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
initializednotification wait loop that fatally rejected any request arriving beforenotifications/initializedserve_innerimmediately after sendingInitializeResult, letting the main service loop handle all messages includinginitializedExpectedInitializedNotificationerror variant fromServerInitializeErrorMotivation
Streamable HTTP requires each JSON-RPC message as a separate POST request. The transport layer cannot guarantee delivery order, so
tools/listcan easily arrive beforenotifications/initialized, causing a fatal HTTP 500 error.The MCP spec uses SHOULD NOT (RFC 2119), not MUST NOT, for pre-initialized messages — allowing exceptions in specific circumstances like HTTP's lack of ordering guarantees.
This aligns with the TypeScript SDK's behavior, which processes requests immediately after
initializewithout gating oninitialized(validated in modelcontextprotocol/typescript-sdk#578).Test plan
server_init_set_level_response_is_empty_resultto handle notifications fromserve_innerdispatchserver_init_succeeds_after_set_level_before_initializedsimilarlyserver_init_rejects_unexpected_message_before_initializedwithserver_init_buffers_request_before_initialized— verifiestools/listbeforeinitializedis processed successfullyserver_init_buffers_multiple_requests_before_initialized— verifies multiple pre-init messages are all processedCloses #783